Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: 0.15.0 release proposal #401

Closed

Conversation

dyladan
Copy link
Member

@dyladan dyladan commented Mar 31, 2021

🚀 Enhancement

  • Other
  • opentelemetry-host-metrics
    • #395 chore: fixing broken links, updating to correct base url, replacing gitter with github discussions (@obecny)

🏠 Internal

📝 Documentation

  • opentelemetry-host-metrics

Committers: 7

@dyladan dyladan requested a review from a team March 31, 2021 15:22
@@ -42,8 +42,8 @@
},
"devDependencies": {
"@opentelemetry/context-async-hooks": "0.18.0",
"@opentelemetry/test-utils": "^0.15.0",
Copy link
Member

@Flarna Flarna Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"@opentelemetry/test-utils": "^0.15.0",
"@opentelemetry/test-utils": "0.15.0",

At least I think this is what renovate bot usually requests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think lerna does this, then the renovate bot undoes it. Maybe lerna can be configured.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I don't care that much, just noticed during scanning the files. Feel free to let the bots do the work.

@@ -45,8 +45,8 @@
"devDependencies": {
"@opentelemetry/context-async-hooks": "0.18.0",
"@opentelemetry/node": "0.18.0",
"@opentelemetry/test-utils": "^0.15.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@@ -49,7 +49,7 @@
"devDependencies": {
"@opentelemetry/context-async-hooks": "0.18.0",
"@opentelemetry/node": "0.18.0",
"@opentelemetry/test-utils": "^0.14.0",
"@opentelemetry/test-utils": "^0.15.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@@ -44,7 +44,7 @@
"@opentelemetry/context-async-hooks": "0.18.0",
"@opentelemetry/node": "0.18.0",
"@opentelemetry/semantic-conventions": "0.18.0",
"@opentelemetry/test-utils": "^0.14.0",
"@opentelemetry/test-utils": "^0.15.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once again

@Flarna
Copy link
Member

Flarna commented Mar 31, 2021

Regarding the compile problem you have here. I think it's caused by the transitive dependency to @hapi/podium from @hapi/hapi. Version 4.1.2 was released recently. If you add "@hapi/podium": "4.1.1" in package.json problem should be gone.

Maybe updating all dependencies to latests solves the problem also.

@dyladan
Copy link
Member Author

dyladan commented Apr 1, 2021

Regarding the compile problem you have here. I think it's caused by the transitive dependency to @hapi/podium from @hapi/hapi. Version 4.1.2 was released recently. If you add "@hapi/podium": "4.1.1" in package.json problem should be gone.

Maybe updating all dependencies to latests solves the problem also.

Thanks. There is also a problem with the transitive dependencies hinted in open-telemetry/opentelemetry-js#2065. The build fails because one of our dependencies (web) depends on another (tracing) with a ^ range. We also depend on a pinned version of tracing. This means tracing is installed twice with nesting in node_modules and typescript fails to install it. Fixing this requires merging the above PR.

Copy link
Contributor

@jtmalinowski jtmalinowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will publish propagator-jaeger@0.15.0, even though it was already published @0.18.2. If we resolve the Hapi issue I can rebase #351 and we'll be good to merge it before publishing this release.

@dyladan
Copy link
Member Author

dyladan commented Apr 1, 2021

Regarding the compile problem you have here. I think it's caused by the transitive dependency to @hapi/podium from @hapi/hapi. Version 4.1.2 was released recently. If you add "@hapi/podium": "4.1.1" in package.json problem should be gone.

Maybe updating all dependencies to latests solves the problem also.

Your suggested fix doesn't work for me at least locally. Skipping lib checks in the tsconfig does, but that feels somehow wrong to me.

@vmarchaud
Copy link
Member

Skipping lib checks in the tsconfig does, but that feels somehow wrong to me.

We could only skip it for Hapi, sadly it sometimes happen and generally requires upstream dependencies to fix their types which can take some time. In our case that shouldn't impact too much i believe

@Flarna
Copy link
Member

Flarna commented Apr 1, 2021

Your suggested fix doesn't work for me at least locally

That's strange as I tried it locally and it worked for me.

@dyladan
Copy link
Member Author

dyladan commented Apr 1, 2021

Your suggested fix doesn't work for me at least locally

That's strange as I tried it locally and it worked for me.

Can you send me the exact package.json? Or open a PR?

@Flarna
Copy link
Member

Flarna commented Apr 1, 2021

Created #407 - let the CI decide

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #401 (18ec496) into main (a40df3c) will decrease coverage by 1.40%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #401      +/-   ##
==========================================
- Coverage   95.83%   94.43%   -1.41%     
==========================================
  Files           1       11      +10     
  Lines          24      431     +407     
  Branches        4       48      +44     
==========================================
+ Hits           23      407     +384     
- Misses          1       24      +23     
Impacted Files Coverage Δ
...s/opentelemetry-propagator-ot-trace/src/version.ts 0.00% <0.00%> (ø)
packages/opentelemetry-host-metrics/src/version.ts 100.00% <100.00%> (ø)
...ackages/opentelemetry-host-metrics/src/stats/si.ts 66.66% <0.00%> (ø)
...ges/opentelemetry-host-metrics/src/stats/common.ts 100.00% <0.00%> (ø)
...ages/opentelemetry-host-metrics/src/BaseMetrics.ts 57.57% <0.00%> (ø)
...ges/opentelemetry-host-metrics/test/metric.test.ts 96.52% <0.00%> (ø)
...metry-propagator-ot-trace/src/OTTracePropagator.ts 95.83% <0.00%> (ø)
packages/opentelemetry-host-metrics/src/enum.ts 100.00% <0.00%> (ø)
... and 2 more

@dyladan
Copy link
Member Author

dyladan commented Apr 1, 2021

Closing in favor of the bot version #409

@dyladan dyladan closed this Apr 1, 2021
@Flarna Flarna deleted the 0.15.0-proposal branch April 26, 2021 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants